feat(web): /workspaces route with host-service terminal viewer#4647
Merged
Conversation
New authed web UI at /workspaces: - workspace list + create form (v2Workspace.list/create) - per-workspace view with a terminal-session dropdown and "New terminal" - xterm-based terminal viewer with mobile fixes (100dvh + visualViewport refit so the soft keyboard no longer covers the prompt) Adds relayQuery to the relay client and remoteControl.listTerminals / remoteControl.createTerminal cloud procedures, which proxy to the host terminal router over the JWT-gated relay path. The terminal stream itself still rides the existing remote-control session machinery.
Replaces the interim remote-control terminal stream with the same path the desktop uses: the host-service /terminal/:terminalId WebSocket reached through the relay's JWT-gated Path A. The browser fetches a Better Auth JWT, gets the relay host URL from a new workspaceTerminal.connection procedure, and speaks the desktop terminal protocol (binary PTY frames + JSON control messages). No remote-control HMAC anywhere in this feature. - new workspaceTerminal cloud router (list / create / connection) - revert the listTerminals/createTerminal additions to the remote-control router; remote-control is left untouched - v2Host.list so the workspace UI can show device names, not machine ids - /workspaces: search + All projects / All devices filters
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a workspace-scoped remote terminal: environment/CSP and deploy wiring, relay auth and client utilities, backend host listing, workspace listing/creation UI, a terminal session management page, and an xterm.js WebTerminal component that streams PTY data via a relay WebSocket. ChangesWorkspace Terminal Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Ready to review this PR? Stage has broken it down into 5 individual chapters for you: 
Chapters generated by Stage for commit 33e6fd3 on May 16, 2026 10:38pm UTC. |
Greptile SummaryThis PR adds an authenticated
Confidence Score: 3/5The list and create terminal flows are likely broken on any live relay due to the unconfirmed JWT scope; the WebSocket streaming path may work independently. The mintRelayJwt function uses scope remote-control for procedures unrelated to the remote-control router, and the PR author flags this as the most probable production breakage. Until the correct scope is confirmed, list and create are broken against a live host. packages/trpc/src/router/workspace-terminal/workspace-terminal.ts — the mintRelayJwt scope claim used for list and create relay calls.
|
| Filename | Overview |
|---|---|
| packages/trpc/src/router/workspace-terminal/workspace-terminal.ts | New tRPC router for workspace terminals; authorization logic is correct, but mintRelayJwt mints with scope remote-control which may be wrong for terminal.listSessions/terminal.createSession. |
| apps/web/src/app/workspaces/[workspaceId]/components/WebTerminal/WebTerminal.tsx | New xterm-based terminal component; auth JWT passed as WebSocket URL query parameter; no reconnect path on disconnect; async setup/teardown and resize-observer logic is sound. |
| packages/trpc/src/router/automation/relay-client.ts | Adds relayQuery (GET) alongside the existing relayMutation (POST); mirrors the tRPC HTTP transport correctly with SuperJSON encoding, timeout/abort, and consistent error handling. |
| packages/trpc/src/router/v2-host/v2-host.ts | Adds a list procedure returning hosts scoped to the active org where the calling user is a member; join and WHERE are correct. |
| apps/web/src/app/workspaces/page.tsx | New workspace list/create page; client-side filtering/sorting is clean; error handling on load and create is present. |
| apps/web/src/app/workspaces/[workspaceId]/page.tsx | New workspace terminal picker page; auto-selects first non-exited terminal and handles visualViewport height for mobile soft-keyboard. |
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
packages/trpc/src/router/workspace-terminal/workspace-terminal.ts:81-87
The JWT minted here for relaying `terminal.listSessions` and `terminal.createSession` uses `scope: "remote-control"`. The existing `remote-control` router uses that scope for `terminal.remoteControl.*` procedures, but those are a different namespace from `terminal.listSessions`/`terminal.createSession`. If the relay's host-service tRPC path enforces scope and these terminal procedures require `terminal.*` or `automation-run`, every `list` and `create` call will return 401/403. The PR description explicitly flags this as the most likely production failure. It's worth resolving before landing.
```suggestion
return mintUserJwt({
userId,
email: owner?.email,
organizationIds: [organizationId],
scope: "terminal", // TODO: confirm the scope the relay's Path A middleware expects for terminal.listSessions / terminal.createSession
ttlSeconds: 300,
});
```
### Issue 2 of 3
apps/web/src/app/workspaces/[workspaceId]/components/WebTerminal/WebTerminal.tsx:164-165
**Auth token exposed in WebSocket URL**
The BetterAuth JWT is appended as a query parameter in the WebSocket URL. While browsers cannot set custom headers on WebSocket upgrades (making query-param auth a common workaround), the token will appear verbatim in relay access logs, browser history, and any CDN or proxy logs that record the full request path. Consider using a dedicated short-lived one-time token for this connection to bound the exposure window.
### Issue 3 of 3
apps/web/src/app/workspaces/[workspaceId]/components/WebTerminal/WebTerminal.tsx:200-210
**No reconnect path after WebSocket close**
`socket.onclose` transitions the state to `"error"` and `socket.onerror` sets the message, but neither exposes a reconnect action. A transient network blip will land the user on the static error banner with no recovery path other than a full page refresh. A reconnect button that re-runs the same setup effect would be a minimal improvement for a terminal UI that explicitly targets mobile.
Reviews (1): Last reviewed commit: "feat(web): route /workspaces terminal th..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+81
to
+87
| return mintUserJwt({ | ||
| userId, | ||
| email: owner?.email, | ||
| organizationIds: [organizationId], | ||
| scope: "remote-control", | ||
| ttlSeconds: 300, | ||
| }); |
There was a problem hiding this comment.
terminal.listSessions and terminal.createSession uses scope: "remote-control". The existing remote-control router uses that scope for terminal.remoteControl.* procedures, but those are a different namespace from terminal.listSessions/terminal.createSession. If the relay's host-service tRPC path enforces scope and these terminal procedures require terminal.* or automation-run, every list and create call will return 401/403. The PR description explicitly flags this as the most likely production failure. It's worth resolving before landing.
Suggested change
| return mintUserJwt({ | |
| userId, | |
| email: owner?.email, | |
| organizationIds: [organizationId], | |
| scope: "remote-control", | |
| ttlSeconds: 300, | |
| }); | |
| return mintUserJwt({ | |
| userId, | |
| email: owner?.email, | |
| organizationIds: [organizationId], | |
| scope: "terminal", // TODO: confirm the scope the relay's Path A middleware expects for terminal.listSessions / terminal.createSession | |
| ttlSeconds: 300, | |
| }); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/trpc/src/router/workspace-terminal/workspace-terminal.ts
Line: 81-87
Comment:
The JWT minted here for relaying `terminal.listSessions` and `terminal.createSession` uses `scope: "remote-control"`. The existing `remote-control` router uses that scope for `terminal.remoteControl.*` procedures, but those are a different namespace from `terminal.listSessions`/`terminal.createSession`. If the relay's host-service tRPC path enforces scope and these terminal procedures require `terminal.*` or `automation-run`, every `list` and `create` call will return 401/403. The PR description explicitly flags this as the most likely production failure. It's worth resolving before landing.
```suggestion
return mintUserJwt({
userId,
email: owner?.email,
organizationIds: [organizationId],
scope: "terminal", // TODO: confirm the scope the relay's Path A middleware expects for terminal.listSessions / terminal.createSession
ttlSeconds: 300,
});
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+164
to
+165
| if (event.data instanceof ArrayBuffer) { | ||
| terminal?.write(new Uint8Array(event.data)); |
There was a problem hiding this comment.
The BetterAuth JWT is appended as a query parameter in the WebSocket URL. While browsers cannot set custom headers on WebSocket upgrades (making query-param auth a common workaround), the token will appear verbatim in relay access logs, browser history, and any CDN or proxy logs that record the full request path. Consider using a dedicated short-lived one-time token for this connection to bound the exposure window.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/src/app/workspaces/[workspaceId]/components/WebTerminal/WebTerminal.tsx
Line: 164-165
Comment:
**Auth token exposed in WebSocket URL**
The BetterAuth JWT is appended as a query parameter in the WebSocket URL. While browsers cannot set custom headers on WebSocket upgrades (making query-param auth a common workaround), the token will appear verbatim in relay access logs, browser history, and any CDN or proxy logs that record the full request path. Consider using a dedicated short-lived one-time token for this connection to bound the exposure window.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+200
to
+210
| }; | ||
|
|
||
| socket.onerror = () => { | ||
| setErrorMessage("WebSocket connection failed."); | ||
| }; | ||
|
|
||
| terminal.onData((data) => { | ||
| const activeSocket = socketRef.current; | ||
| if (activeSocket?.readyState === WebSocket.OPEN) { | ||
| activeSocket.send(JSON.stringify({ type: "input", data })); | ||
| } |
There was a problem hiding this comment.
socket.onclose transitions the state to "error" and socket.onerror sets the message, but neither exposes a reconnect action. A transient network blip will land the user on the static error banner with no recovery path other than a full page refresh. A reconnect button that re-runs the same setup effect would be a minimal improvement for a terminal UI that explicitly targets mobile.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/src/app/workspaces/[workspaceId]/components/WebTerminal/WebTerminal.tsx
Line: 200-210
Comment:
**No reconnect path after WebSocket close**
`socket.onclose` transitions the state to `"error"` and `socket.onerror` sets the message, but neither exposes a reconnect action. A transient network blip will land the user on the static error banner with no recovery path other than a full page refresh. A reconnect button that re-runs the same setup effect would be a minimal improvement for a terminal UI that explicitly targets mobile.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/trpc/src/router/automation/relay-client.ts (1)
103-158: 💤 Low valueConsider extracting shared response-parsing logic.
The response handling in
relayQuery(lines 128–157) duplicatesrelayMutation(lines 65–94). Extracting aparseRelayResponse<T>(response, rawBody)helper would reduce maintenance overhead.This is optional given the limited scope of this module.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/trpc/src/router/automation/relay-client.ts` around lines 103 - 158, relayQuery contains duplicated response-parsing/error-handling logic also present in relayMutation; extract that shared logic into a helper like parseRelayResponse<T>(response: Response, rawBody: string): T that performs JSON.parse, validates the TrpcEnvelope/result.data shape, throws RelayDispatchError with response.status and rawBody on invalid JSON or missing data, and returns SuperJSON.deserialize(parsed.result.data) typed as T; then replace the duplicated blocks in both relayQuery and relayMutation to call parseRelayResponse<TOutput>(response, rawBody) (or the appropriate generic) and return its result to remove redundancy and centralize error messages.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/web/src/app/workspaces/`[workspaceId]/components/WebTerminal/WebTerminal.tsx:
- Around line 202-204: The WebSocket onerror handler currently only calls
setErrorMessage which leaves the connection state stuck at "connecting"; update
the onerror handler (socket.onerror) to also update the connection state by
calling the state setter used in this component (e.g.,
setConnectionState("failed") or setConnectionState("disconnected")), so the UI
no longer shows "connecting" after an error; keep the existing setErrorMessage
call and add the connection-state update in the same handler.
In `@apps/web/src/app/workspaces/`[workspaceId]/page.tsx:
- Around line 51-56: The effect that initializes selection only runs when
selectedTerminalId is falsy, so if the currently selected terminal disappears on
a refresh the selection is never reconciled; update the useEffect (watching
terminals and selectedTerminalId) to also check whether the current
selectedTerminalId exists in the new terminals list and if not pick a
replacement (prefer the first non-exited terminal or terminals[0]) and call
setSelectedTerminalId with that id; reference the existing identifiers
useEffect, selectedTerminalId, terminals, setSelectedTerminalId, and the earlier
logic that chooses first = terminals.find((t)=>!t.exited) ?? terminals[0].
In `@apps/web/src/app/workspaces/page.tsx`:
- Around line 169-204: The inputs and selects for creating/filtering workspaces
(state variables name, branch, projectId, hostId and the iterated projects and
hosts lists using project.id and host.machineId) lack programmatic labels; add
accessible labels by either adding <label htmlFor="..."> elements and matching
id attributes on the corresponding input/select elements (e.g., ids like
workspace-name, workspace-branch, workspace-project, workspace-host) or by
adding clear aria-label attributes if visual labels are not desired; ensure
option lists remain unchanged and use hostLabel(host) for visible option text
while the select has an associated label for screen readers.
In `@packages/trpc/src/router/workspace-terminal/workspace-terminal.ts`:
- Around line 75-88: In mintRelayJwt, the DB lookup can return no user so owner
may be undefined and email passed as undefined to mintUserJwt; update
mintRelayJwt to guard the lookup result (the owner variable from the select on
users) and handle the missing user case before calling mintUserJwt — e.g., throw
a clear error or return a controlled failure when owner is falsy, or
fetch/derive a fallback email if appropriate, ensuring mintUserJwt is never
called with email: undefined.
---
Nitpick comments:
In `@packages/trpc/src/router/automation/relay-client.ts`:
- Around line 103-158: relayQuery contains duplicated
response-parsing/error-handling logic also present in relayMutation; extract
that shared logic into a helper like parseRelayResponse<T>(response: Response,
rawBody: string): T that performs JSON.parse, validates the
TrpcEnvelope/result.data shape, throws RelayDispatchError with response.status
and rawBody on invalid JSON or missing data, and returns
SuperJSON.deserialize(parsed.result.data) typed as T; then replace the
duplicated blocks in both relayQuery and relayMutation to call
parseRelayResponse<TOutput>(response, rawBody) (or the appropriate generic) and
return its result to remove redundancy and centralize error messages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9619d1f3-e836-4a64-bb6e-b26e3b583fb2
📒 Files selected for processing (9)
apps/web/src/app/workspaces/[workspaceId]/components/WebTerminal/WebTerminal.tsxapps/web/src/app/workspaces/[workspaceId]/components/WebTerminal/index.tsapps/web/src/app/workspaces/[workspaceId]/page.tsxapps/web/src/app/workspaces/page.tsxpackages/trpc/src/root.tspackages/trpc/src/router/automation/relay-client.tspackages/trpc/src/router/v2-host/v2-host.tspackages/trpc/src/router/workspace-terminal/index.tspackages/trpc/src/router/workspace-terminal/workspace-terminal.ts
Comment on lines
+202
to
+204
| socket.onerror = () => { | ||
| setErrorMessage("WebSocket connection failed."); | ||
| }; |
Contributor
There was a problem hiding this comment.
Set connection state on WebSocket error events.
onerror sets only the message; state can remain "connecting" longer than needed.
Suggested fix
socket.onerror = () => {
setErrorMessage("WebSocket connection failed.");
+ setState("error");
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| socket.onerror = () => { | |
| setErrorMessage("WebSocket connection failed."); | |
| }; | |
| socket.onerror = () => { | |
| setErrorMessage("WebSocket connection failed."); | |
| setState("error"); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/web/src/app/workspaces/`[workspaceId]/components/WebTerminal/WebTerminal.tsx
around lines 202 - 204, The WebSocket onerror handler currently only calls
setErrorMessage which leaves the connection state stuck at "connecting"; update
the onerror handler (socket.onerror) to also update the connection state by
calling the state setter used in this component (e.g.,
setConnectionState("failed") or setConnectionState("disconnected")), so the UI
no longer shows "connecting" after an error; keep the existing setErrorMessage
call and add the connection-state update in the same handler.
Comment on lines
+51
to
+56
| useEffect(() => { | ||
| if (selectedTerminalId || !terminals) return; | ||
| const first = | ||
| terminals.find((terminal) => !terminal.exited) ?? terminals[0]; | ||
| if (first) setSelectedTerminalId(first.terminalId); | ||
| }, [terminals, selectedTerminalId]); |
Contributor
There was a problem hiding this comment.
Reconcile selectedTerminalId against refreshed terminal lists.
Selection is only initialized when null; it is not corrected when the current terminal is missing after reload.
Suggested fix
useEffect(() => {
- if (selectedTerminalId || !terminals) return;
- const first =
- terminals.find((terminal) => !terminal.exited) ?? terminals[0];
- if (first) setSelectedTerminalId(first.terminalId);
+ if (!terminals) return;
+ const stillExists =
+ selectedTerminalId !== null &&
+ terminals.some((terminal) => terminal.terminalId === selectedTerminalId);
+ if (stillExists) return;
+
+ const first =
+ terminals.find((terminal) => !terminal.exited) ?? terminals[0] ?? null;
+ setSelectedTerminalId(first?.terminalId ?? null);
}, [terminals, selectedTerminalId]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/app/workspaces/`[workspaceId]/page.tsx around lines 51 - 56, The
effect that initializes selection only runs when selectedTerminalId is falsy, so
if the currently selected terminal disappears on a refresh the selection is
never reconciled; update the useEffect (watching terminals and
selectedTerminalId) to also check whether the current selectedTerminalId exists
in the new terminals list and if not pick a replacement (prefer the first
non-exited terminal or terminals[0]) and call setSelectedTerminalId with that
id; reference the existing identifiers useEffect, selectedTerminalId, terminals,
setSelectedTerminalId, and the earlier logic that chooses first =
terminals.find((t)=>!t.exited) ?? terminals[0].
Comment on lines
+169
to
+204
| <input | ||
| value={name} | ||
| onChange={(event) => setName(event.target.value)} | ||
| placeholder="Name" | ||
| className="rounded-md border bg-transparent px-3 py-2 text-sm" | ||
| /> | ||
| <input | ||
| value={branch} | ||
| onChange={(event) => setBranch(event.target.value)} | ||
| placeholder="Branch" | ||
| className="rounded-md border bg-transparent px-3 py-2 text-sm" | ||
| /> | ||
| <select | ||
| value={projectId} | ||
| onChange={(event) => setProjectId(event.target.value)} | ||
| className="rounded-md border bg-transparent px-3 py-2 text-sm" | ||
| > | ||
| <option value="">Select project…</option> | ||
| {projects.map((project) => ( | ||
| <option key={project.id} value={project.id}> | ||
| {project.name} | ||
| </option> | ||
| ))} | ||
| </select> | ||
| <select | ||
| value={hostId} | ||
| onChange={(event) => setHostId(event.target.value)} | ||
| className="rounded-md border bg-transparent px-3 py-2 text-sm" | ||
| > | ||
| <option value="">Select device…</option> | ||
| {hosts.map((host) => ( | ||
| <option key={host.machineId} value={host.machineId}> | ||
| {hostLabel(host)} | ||
| </option> | ||
| ))} | ||
| </select> |
Contributor
There was a problem hiding this comment.
Add explicit labels for form and filter controls.
These controls are missing programmatic labels, which makes the create/filter flows hard to use with assistive tech.
Suggested fix
+<label htmlFor="workspace-name" className="sr-only">Workspace name</label>
<input
+ id="workspace-name"
value={name}
onChange={(event) => setName(event.target.value)}
placeholder="Name"
className="rounded-md border bg-transparent px-3 py-2 text-sm"
/>
+<label htmlFor="workspace-branch" className="sr-only">Branch</label>
<input
+ id="workspace-branch"
value={branch}
onChange={(event) => setBranch(event.target.value)}
placeholder="Branch"
className="rounded-md border bg-transparent px-3 py-2 text-sm"
/>
+<label htmlFor="workspace-project" className="sr-only">Project</label>
<select
+ id="workspace-project"
value={projectId}
onChange={(event) => setProjectId(event.target.value)}
className="rounded-md border bg-transparent px-3 py-2 text-sm"
>
+<label htmlFor="workspace-host" className="sr-only">Device</label>
<select
+ id="workspace-host"
value={hostId}
onChange={(event) => setHostId(event.target.value)}
className="rounded-md border bg-transparent px-3 py-2 text-sm"
>Also applies to: 224-253
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/app/workspaces/page.tsx` around lines 169 - 204, The inputs and
selects for creating/filtering workspaces (state variables name, branch,
projectId, hostId and the iterated projects and hosts lists using project.id and
host.machineId) lack programmatic labels; add accessible labels by either adding
<label htmlFor="..."> elements and matching id attributes on the corresponding
input/select elements (e.g., ids like workspace-name, workspace-branch,
workspace-project, workspace-host) or by adding clear aria-label attributes if
visual labels are not desired; ensure option lists remain unchanged and use
hostLabel(host) for visible option text while the select has an associated label
for screen readers.
Contributor
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/web/src/app/workspaces/[workspaceId]/page.tsx">
<violation number="1" location="apps/web/src/app/workspaces/[workspaceId]/page.tsx:34">
P2: `loadTerminals` updates state unconditionally, so overlapping calls can race and let stale responses overwrite newer terminal data.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic
| workspaceId, | ||
| }); | ||
| setLoadError(null); | ||
| setTerminals( |
Contributor
There was a problem hiding this comment.
P2: loadTerminals updates state unconditionally, so overlapping calls can race and let stale responses overwrite newer terminal data.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/app/workspaces/[workspaceId]/page.tsx, line 34:
<comment>`loadTerminals` updates state unconditionally, so overlapping calls can race and let stale responses overwrite newer terminal data.</comment>
<file context>
@@ -0,0 +1,159 @@
+ workspaceId,
+ });
+ setLoadError(null);
+ setTerminals(
+ result.map((terminal) => ({
+ terminalId: terminal.terminalId,
</file context>
Drops the cloud-side workspaceTerminal tRPC proxy. The web app now talks to the relay directly — the same path the desktop uses: - terminal list/create: a browser fetch client (trpc/host-client.ts) to the relay's /hosts/<routingKey>/trpc endpoint, JWT-authed - terminal stream: WebSocket straight to the relay, no cloud hop - the user JWT comes from Better Auth /api/auth/token (trpc/auth-token.ts) Removes the workspace-terminal router and the relayQuery helper added earlier; remote-control is left untouched. Adds NEXT_PUBLIC_RELAY_URL (env, deploy workflows, .env.example) and the relay HTTP origin to the web CSP connect-src.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/next.config.ts`:
- Around line 24-33: The CSP origins are being derived from
process.env.RELAY_URL while the browser uses process.env.NEXT_PUBLIC_RELAY_URL;
update the logic that computes relayWsOrigin and relayHttpOrigin (and the other
occurrence around the same block referenced as the second occurrence) to prefer
process.env.NEXT_PUBLIC_RELAY_URL as the source-of-truth (use
NEXT_PUBLIC_RELAY_URL if present, fall back to RELAY_URL if not), ensure ws/http
scheme conversion is applied to the chosen value, and keep the same production
default fallbacks ("wss://relay.superset.sh" / "https://relay.superset.sh") and
null behavior when neither is set.
In `@apps/web/src/app/workspaces/`[workspaceId]/page.tsx:
- Around line 54-80: The effect that resolves the workspace can leave stale
terminal state when resolution fails or races; update the effect in the
workspace loading logic (the async useEffect that calls
trpcClient.organization.getActive and trpcClient.v2Workspace.getFromHost, and
the similar block later) to clear any previous terminal context immediately when
starting and on error: call setRoutingKey(null) and setSelectedTerminalId(null)
before attempting lookups, and also ensure the catch block sets
setRoutingKey(null) and setSelectedTerminalId(null) in addition to
setTerminals([]) and setLoadError(...); this guarantees the page won't keep
rendering an old terminal while workspace resolution is in flight or fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 94d532ea-033a-4813-900c-0e0df2b37036
📒 Files selected for processing (9)
.env.example.github/workflows/deploy-preview.yml.github/workflows/deploy-production.ymlapps/web/next.config.tsapps/web/src/app/workspaces/[workspaceId]/components/WebTerminal/WebTerminal.tsxapps/web/src/app/workspaces/[workspaceId]/page.tsxapps/web/src/env.tsapps/web/src/trpc/auth-token.tsapps/web/src/trpc/host-client.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/app/workspaces/[workspaceId]/components/WebTerminal/WebTerminal.tsx
Comment on lines
24
to
+33
| const relayWsOrigin = process.env.RELAY_URL | ||
| ? new URL(process.env.RELAY_URL).origin.replace(/^http/, "ws") | ||
| : isProduction | ||
| ? "wss://relay.superset.sh" | ||
| : null; | ||
| const relayHttpOrigin = process.env.RELAY_URL | ||
| ? new URL(process.env.RELAY_URL).origin | ||
| : isProduction | ||
| ? "https://relay.superset.sh" | ||
| : null; |
Contributor
There was a problem hiding this comment.
Use NEXT_PUBLIC_RELAY_URL as the CSP relay source-of-truth.
connect-src relay origins are derived from RELAY_URL, while browser calls use NEXT_PUBLIC_RELAY_URL. If these differ (or only the public var is set), relay requests can be blocked by CSP.
Suggested fix
-const relayWsOrigin = process.env.RELAY_URL
- ? new URL(process.env.RELAY_URL).origin.replace(/^http/, "ws")
- : isProduction
- ? "wss://relay.superset.sh"
- : null;
-const relayHttpOrigin = process.env.RELAY_URL
- ? new URL(process.env.RELAY_URL).origin
- : isProduction
- ? "https://relay.superset.sh"
- : null;
+const relayBaseUrl =
+ process.env.NEXT_PUBLIC_RELAY_URL ?? process.env.RELAY_URL;
+const relayHttpOrigin = relayBaseUrl
+ ? new URL(relayBaseUrl).origin
+ : isProduction
+ ? "https://relay.superset.sh"
+ : null;
+const relayWsOrigin = relayHttpOrigin
+ ? relayHttpOrigin.replace(/^http/, "ws")
+ : null;Also applies to: 42-42
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/next.config.ts` around lines 24 - 33, The CSP origins are being
derived from process.env.RELAY_URL while the browser uses
process.env.NEXT_PUBLIC_RELAY_URL; update the logic that computes relayWsOrigin
and relayHttpOrigin (and the other occurrence around the same block referenced
as the second occurrence) to prefer process.env.NEXT_PUBLIC_RELAY_URL as the
source-of-truth (use NEXT_PUBLIC_RELAY_URL if present, fall back to RELAY_URL if
not), ensure ws/http scheme conversion is applied to the chosen value, and keep
the same production default fallbacks ("wss://relay.superset.sh" /
"https://relay.superset.sh") and null behavior when neither is set.
Comment on lines
+54
to
+80
| useEffect(() => { | ||
| (async () => { | ||
| try { | ||
| const organization = await trpcClient.organization.getActive.query(); | ||
| if (!organization) { | ||
| setLoadError("No active organization."); | ||
| setTerminals([]); | ||
| return; | ||
| } | ||
| const workspace = await trpcClient.v2Workspace.getFromHost.query({ | ||
| organizationId: organization.id, | ||
| id: workspaceId, | ||
| }); | ||
| if (!workspace) { | ||
| setLoadError("Workspace not found."); | ||
| setTerminals([]); | ||
| return; | ||
| } | ||
| const key = buildHostRoutingKey(organization.id, workspace.hostId); | ||
| setRoutingKey(key); | ||
| await loadTerminals(key); | ||
| } catch (caught) { | ||
| setLoadError(caught instanceof Error ? caught.message : String(caught)); | ||
| setTerminals([]); | ||
| } | ||
| })(); | ||
| }, [workspaceId, loadTerminals]); |
Contributor
There was a problem hiding this comment.
Reset stale terminal context before/after failed workspace resolution.
The loader effect can leave previous routingKey/selectedTerminalId active when navigation races or lookup fails, so the page may continue rendering an old terminal context on the new route.
Suggested fix
useEffect(() => {
+ let cancelled = false;
+ setRoutingKey(null);
+ setSelectedTerminalId(null);
+ setTerminals(null);
+ setLoadError(null);
+
(async () => {
try {
const organization = await trpcClient.organization.getActive.query();
+ if (cancelled) return;
if (!organization) {
+ setRoutingKey(null);
+ setSelectedTerminalId(null);
setLoadError("No active organization.");
setTerminals([]);
return;
}
const workspace = await trpcClient.v2Workspace.getFromHost.query({
organizationId: organization.id,
id: workspaceId,
});
+ if (cancelled) return;
if (!workspace) {
+ setRoutingKey(null);
+ setSelectedTerminalId(null);
setLoadError("Workspace not found.");
setTerminals([]);
return;
}
const key = buildHostRoutingKey(organization.id, workspace.hostId);
+ if (cancelled) return;
setRoutingKey(key);
await loadTerminals(key);
} catch (caught) {
+ if (cancelled) return;
+ setRoutingKey(null);
+ setSelectedTerminalId(null);
setLoadError(caught instanceof Error ? caught.message : String(caught));
setTerminals([]);
}
})();
+ return () => {
+ cancelled = true;
+ };
}, [workspaceId, loadTerminals]);Also applies to: 173-179
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/app/workspaces/`[workspaceId]/page.tsx around lines 54 - 80, The
effect that resolves the workspace can leave stale terminal state when
resolution fails or races; update the effect in the workspace loading logic (the
async useEffect that calls trpcClient.organization.getActive and
trpcClient.v2Workspace.getFromHost, and the similar block later) to clear any
previous terminal context immediately when starting and on error: call
setRoutingKey(null) and setSelectedTerminalId(null) before attempting lookups,
and also ensure the catch block sets setRoutingKey(null) and
setSelectedTerminalId(null) in addition to setTerminals([]) and
setLoadError(...); this guarantees the page won't keep rendering an old terminal
while workspace resolution is in flight or fails.
Contributor
There was a problem hiding this comment.
3 issues found across 13 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/web/src/trpc/host-client.ts">
<violation number="1" location="apps/web/src/trpc/host-client.ts:41">
P2: Include relay/host response text in non-OK errors so failure diagnostics are not lost.
(Based on your team's feedback about preserving useful diagnostic context in errors.) [FEEDBACK_USED]</violation>
<violation number="2" location="apps/web/src/trpc/host-client.ts:44">
P3: Wrap JSON parsing so malformed relay responses throw a procedure-scoped error instead of a generic SyntaxError.
(Based on your team's feedback about preserving useful diagnostic context in errors.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/web/next.config.ts">
<violation number="1" location="apps/web/next.config.ts:29">
P2: Derive relay CSP origins from the same public relay URL used by browser calls; otherwise terminal and host tRPC requests can be blocked by `connect-src` when `RELAY_URL` and `NEXT_PUBLIC_RELAY_URL` diverge.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic
| body: method === "POST" ? JSON.stringify(encoded) : undefined, | ||
| }); | ||
| if (!response.ok) { | ||
| throw new Error(`host ${procedure} failed (${response.status})`); |
Contributor
There was a problem hiding this comment.
P2: Include relay/host response text in non-OK errors so failure diagnostics are not lost.
(Based on your team's feedback about preserving useful diagnostic context in errors.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/trpc/host-client.ts, line 41:
<comment>Include relay/host response text in non-OK errors so failure diagnostics are not lost.
(Based on your team's feedback about preserving useful diagnostic context in errors.) </comment>
<file context>
@@ -0,0 +1,67 @@
+ body: method === "POST" ? JSON.stringify(encoded) : undefined,
+ });
+ if (!response.ok) {
+ throw new Error(`host ${procedure} failed (${response.status})`);
+ }
+
</file context>
| : isProduction | ||
| ? "wss://relay.superset.sh" | ||
| : null; | ||
| const relayHttpOrigin = process.env.RELAY_URL |
Contributor
There was a problem hiding this comment.
P2: Derive relay CSP origins from the same public relay URL used by browser calls; otherwise terminal and host tRPC requests can be blocked by connect-src when RELAY_URL and NEXT_PUBLIC_RELAY_URL diverge.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/next.config.ts, line 29:
<comment>Derive relay CSP origins from the same public relay URL used by browser calls; otherwise terminal and host tRPC requests can be blocked by `connect-src` when `RELAY_URL` and `NEXT_PUBLIC_RELAY_URL` diverge.</comment>
<file context>
@@ -16,16 +16,21 @@ const isProduction = process.env.NODE_ENV === "production";
: isProduction
? "wss://relay.superset.sh"
: null;
+const relayHttpOrigin = process.env.RELAY_URL
+ ? new URL(process.env.RELAY_URL).origin
+ : isProduction
</file context>
| throw new Error(`host ${procedure} failed (${response.status})`); | ||
| } | ||
|
|
||
| const parsed = (await response.json()) as { result?: { data?: unknown } }; |
Contributor
There was a problem hiding this comment.
P3: Wrap JSON parsing so malformed relay responses throw a procedure-scoped error instead of a generic SyntaxError.
(Based on your team's feedback about preserving useful diagnostic context in errors.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/trpc/host-client.ts, line 44:
<comment>Wrap JSON parsing so malformed relay responses throw a procedure-scoped error instead of a generic SyntaxError.
(Based on your team's feedback about preserving useful diagnostic context in errors.) </comment>
<file context>
@@ -0,0 +1,67 @@
+ throw new Error(`host ${procedure} failed (${response.status})`);
+ }
+
+ const parsed = (await response.json()) as { result?: { data?: unknown } };
+ if (!parsed.result || parsed.result.data === undefined) {
+ throw new Error(`host ${procedure}: malformed relay response`);
</file context>
Mirrors the desktop's useRelayUrl: getRelayUrl() reads the `relay-url-override` flag payload and falls back to NEXT_PUBLIC_RELAY_URL. Used for both the host tRPC client and the terminal WebSocket.
This was referenced May 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/workspaces: create + list workspaces (search, project & device filters), per-workspace terminal-session picker, and an xterm terminal./terminal/:terminalIdWebSocket over the relay's JWT-gated path — the same path the desktop uses. No remote-control HMAC machinery.Why / Context
The web app had no real terminal UI — only the anonymous-viewer
remote-controlshare-link flow. We want authed users to reach their own workspace terminals from the browser, using the host-service + relay path the desktop already relies on, rather than the bearer-token remote-control mechanism.How It Works
/api/auth/token), gets the relay host URL from a newworkspaceTerminal.connectionprocedure, and openswss://<relay>/hosts/<routingKey>/terminal/<terminalId>?token=<jwt>. The relay's Path A middleware verifies the JWT + host access; the host-service streams the PTY. Wire protocol mirrors the desktop transport (binary frames = PTY bytes, JSON control messages).workspaceTerminal.list/createproxy to the host'sterminal.listSessions/terminal.createSessiontRPC over the relay (cloud mints a short-lived JWT).v2Host.listadded so the workspace UI shows device names instead of raw machine IDs.listTerminals/createTerminaladditions briefly made to theremote-controlrouter were reverted — that router is untouched.Manual QA Checklist
/workspaceslists real workspaces; search + project/device filters narrow correctly/workspaces/[id]lists host terminal sessions; "New terminal" creates oneTesting
bun run lint— cleanbun run typecheck— clean (28/28 packages)Design Decisions
/terminal/:terminalIdover remote-control: remote-control is a bearer-token mechanism built for anonymous viewers; authed web users should use the relay's JWT-gated path like the desktop. Streaming now does exactly that.connectionprocedure returns the relay URL: the web app has noNEXT_PUBLIC_RELAY_URL; the cloud ownsRELAY_URLand resolves the routing key, so a tiny procedure hands back the WS base.Known Limitations
scopeclaim if the host rejects it forterminal.*.v2Workspacesrow only; it does not run the host-side worktree-creation saga, so a created workspace has no real worktree yet. Listing and the terminal view for existing workspaces are the solid paths.list/createproxy through the cloud rather than calling the host directly like the desktop; the terminal stream is direct.Follow-ups
Summary by cubic
Adds an authenticated /workspaces route with workspace list/create and a per-workspace terminal viewer. The web app connects to the relay directly for terminal WebSocket and host-service tRPC with a Better Auth JWT, and supports a PostHog relay URL override.
New Features
/workspaceslist with search and project/device filters, plus create form;/workspaces/[id]with session picker and “New terminal”.@xterm/xtermviewer (with@xterm/addon-fit) and mobile visualViewport refit so the soft keyboard doesn’t cover the prompt./api/auth/token), resolves relay fromNEXT_PUBLIC_RELAY_URLor therelay-url-overridePostHog flag, and connects towss://…/hosts/<routingKey>/terminal/<terminalId>; session list/create call relay…/trpc/terminal.*directly (JWT). Protocol mirrors desktop (binary PTY + JSON control).v2Host.listso the UI shows device names instead of machine IDs.Refactors
workspaceTerminalproxy; addedtrpc/host-client.tsfor direct browser → relay host tRPC.NEXT_PUBLIC_RELAY_URL(env + deploy workflows) and allowed the relay HTTP origin in CSPconnect-src.Written for commit 33e6fd3. Summary will update on new commits. Review in cubic
Summary by CodeRabbit